Skip to content
This repository was archived by the owner on Oct 25, 2021. It is now read-only.

Gamzat_Asadulaev#16

Open
GamzatAsadulaev wants to merge 35 commits intomasterfrom
feature/GamzatAsadulaev
Open

Gamzat_Asadulaev#16
GamzatAsadulaev wants to merge 35 commits intomasterfrom
feature/GamzatAsadulaev

Conversation

@GamzatAsadulaev
Copy link
Collaborator

No description provided.

@GamzatAsadulaev GamzatAsadulaev changed the title GamzatAsadulaev_homework1 feature/GamzatAsadulaev Jul 7, 2021
@NikolaevArtem NikolaevArtem added the HW_1 Good to go label Jul 9, 2021
@NikolaevArtem NikolaevArtem changed the title feature/GamzatAsadulaev Gamzat_Asadulaev Jul 11, 2021
@GamzatAsadulaev GamzatAsadulaev force-pushed the feature/GamzatAsadulaev branch from f766609 to bfb5444 Compare July 14, 2021 13:46
@GamzatAsadulaev GamzatAsadulaev force-pushed the feature/GamzatAsadulaev branch from 3919f28 to ba3855b Compare July 15, 2021 21:16
@GamzatAsadulaev GamzatAsadulaev force-pushed the feature/GamzatAsadulaev branch from 94f3e80 to 05f80cc Compare July 16, 2021 09:42
@GamzatAsadulaev GamzatAsadulaev added the readyForReview Sign for Artem to take a look label Jul 16, 2021
@NikolaevArtem NikolaevArtem added HW_2 style Style fix needed and removed readyForReview Sign for Artem to take a look labels Jul 17, 2021
@GamzatAsadulaev GamzatAsadulaev removed the style Style fix needed label Sep 20, 2021
@NikolaevArtem NikolaevArtem removed readyForReview Sign for Artem to take a look readyForStudentReview labels Sep 21, 2021
Comment on lines +16 to +17
testImplementation 'org.hamcrest:hamcrest:2.2'
testImplementation 'org.hamcrest:hamcrest-library:2.2'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hamcrest matchers are nice!

public class Main {

public static void main(String[] args) {
new Game().run();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On player's field hits/misses are not seen
image

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Input is not quite simple... Automatic ship placement is nice!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opt: Your utils folder is oversized, some classes should be moved to other places

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite good! You show good knowledge of Java Core and Java libraries, code is mostly clean and easy to read. Abstractions are chosen correctly, also good architecture. I didn't see any bugs in the game, but it would look better if you refactor the interface.

Approved!

Comment on lines +11 to +18
int counter = 0;
for (int i = 0; i < player.getMonitorField().getField().length; i++) {
for (int j = 0; j < player.getMonitorField().getField().length; j++) {
if (player.getMonitorField().getField()[i][j].getState() == Position.State.HIT) {
counter++;
}
}
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opt: that's not really efficient... you iterate through all field each turn. There are much easier ways to achieve the same result


import java.util.*;

public final class Book {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your class is mutable

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, there's no description of a immutable class


@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
public @interface FilePath {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good example!

Comment on lines +11 to +12
KittenToCatFunction<Kitten, Cat> function = (k -> new Cat(k.getName(),
k.getAge() + 1, k.getWeight() * 3));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opt: introduce such fields and such lambda, that you'll be able to perform type conversion (as example, favourite color to favorite toy, different classes)


@ParameterizedTest
@MethodSource("invalidCoordinates")
void givenInvalidCoordinate_whenRun_thenCheckReturn(Coordinate coordinate) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice test cases names!

Copy link
Owner

@NikolaevArtem NikolaevArtem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well-done! There are some fixes TBD in HW_3, but I'll approve your homework now with the promise that you'll fix it.

Good job, homework is approved!

public Book(String name, int age, String... authors) {
this.title = name;
this.year = age;
this.authors = authors;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still mutable...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants